Implement RFC 9207 issuer validation in ClientOAuthProvider#1605
Implement RFC 9207 issuer validation in ClientOAuthProvider#1605mikekistler wants to merge 9 commits into
Conversation
| /// the redirect URI callback and return them in an <see cref="AuthorizationResult"/>. | ||
| /// </para> | ||
| /// </remarks> | ||
| public Func<Uri, Uri, CancellationToken, Task<AuthorizationResult?>>? AuthorizationCallbackHandler { get; set; } |
There was a problem hiding this comment.
Given that this is going to be new in new major version, 2.0, I'd almost take a breaking change to AuthorizationRedirectDelegate over a mutually exclusive new callback. If we like the new name better, maybe just Obsolete the AuthorizationRedirectDelegate property and type?
Also, while I usually lean towards liking Funcs over custom delegate types, I think it's useful when there's multiple parameters of the same type like Uri. I'm also wondering if we shouldn't take a context object instead to avoid future breaking changes. Then maybe we could stick with the Func.
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
|
@tarekgh @PranavSenthilnathan I updated this to make the breaking API change I suggested in my earlier comment. Let me know what you think. |
Resolves the semantic merge conflict: main's step-up scope tests used the removed AuthorizationRedirectDelegate API. Migrated them to the new AuthorizationCallbackHandler / AuthorizationCallbackContext API.
…elegate The AuthorizationRedirectDelegate type and ClientOAuthOptions.AuthorizationRedirectDelegate property were removed in favor of the new AuthorizationCallbackHandler API. Add baseline suppressions (CP0001/CP0002) so the Release pack's package validation against 1.3.0 passes.
@halter73 I have left a few comments, but in general LGTM. |
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
|
@halter73 could you please resolve the conflict to the see the CI results? |
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
|
I addressed the PR feedback, but wait a minute before rereviewing. I'm going to add the AuthorizationRedirectDelegate back temporarily as an obsolete API for back-compat purposes. |
4 participants
Summary
Implements SEP-2468 — RFC 9207 issuer (
iss) parameter validation in the OAuth authorization flow.Closes #1571
Changes
AuthorizationResult— New class that returns both the authorization code and the validated issuer URI from the authorization redirect.ClientOAuthProvider— Validates theissparameter in authorization responses per RFC 9207, and validates that the authorization server metadataissuerfield matches the expected URI per RFC 8414 Section 3.3.AuthorizationRedirectDelegate— Updated signature to returnAuthorizationResult(containing issuer) instead of just a string code.AuthorizationServerMetadata— AddedIssuerproperty.ClientOAuthOptions— Updated to accommodate the new authorization result type.AuthorizationResultreturn type.RFC 9207 Behavior
issparameter in the authorization response, the client validates it matches the expected authorization server issuer.issueris validated against the expected URI per RFC 8414 Section 3.3.